Security Fix: Clear-text Logging of Sensitive Information

Alert Number: #71
Severity: High (Error)
Rule: go/clear-text-logging
CWE: CWE-312, CWE-315, CWE-359

Vulnerability Description

The secrets validation function was logging sensitive information (actual secret values) in both error messages and log statements when validation failed. This occurs in pkg/workflow/secrets_validation.go at line 22, where the error message included got: %q with the actual value, and line 21 where the log statement included the value parameter.

When a user provides an invalid secret expression (e.g., a plaintext value instead of ${{ secrets.NAME }}), the actual sensitive value would be:

1. Logged to the application logs via secretsValidationLog.Printf
2. Included in the error message returned to the caller
3. Potentially stored in workflow run logs or other output streams

This violates the security principle of never logging sensitive data in clear text.

Fix Applied

Changes to pkg/workflow/secrets_validation.go:

1. Removed the got: %q format specifier and value argument from the error message (line 22)
2. Removed the value parameter from the log statement (line 21)
3. Error messages now only include the key name and expected format examples, never the actual value

Before:

secretsValidationLog.Printf("Invalid secret expression for key %s: %s", key, value)
return fmt.Errorf("jobs.secrets.%s must be a GitHub Actions expression with secrets reference (e.g., '${{ secrets.MY_SECRET }}' or '${{ secrets.SECRET1 || secrets.SECRET2 }}'), got: %q", key, value)

After:

secretsValidationLog.Printf("Invalid secret expression for key %s", key)
return fmt.Errorf("jobs.secrets.%s must be a GitHub Actions expression with secrets reference (e.g., '${{ secrets.MY_SECRET }}' or '${{ secrets.SECRET1 || secrets.SECRET2 }}')", key)

Changes to pkg/workflow/secrets_validation_test.go:

1. Updated test cases to verify that sensitive values are NOT included in error messages
2. Added notExpectedInErrs field to test structure to explicitly verify sensitive data exclusion
3. Updated test documentation to clarify the security requirement
4. All tests now verify that error messages do NOT contain the actual secret values

Security Best Practices Applied

✅ Principle of Least Privilege for Logging: Only log the minimum information needed for debugging (key name), never the sensitive value itself

✅ Defense in Depth: Even though the value might be invalid, we treat all input as potentially sensitive

✅ Secure by Default: Error messages are helpful but never expose sensitive data

✅ Testable Security: Added explicit tests to verify sensitive data is not leaked

Testing Considerations

All existing tests have been updated and pass successfully:

• ✅ TestSecretsExpressionPattern - Pattern matching still works correctly
• ✅ TestValidateSecretsExpressionErrorMessages - Error messages are descriptive but do NOT contain sensitive values
• ✅ TestValidateSecretsExpressionWithDifferentKeys - Validation logic unchanged
• ✅ TestSecretsValidationEdgeCases - Edge cases still handled correctly

Run tests with:

go test -v ./pkg/workflow -run TestValidateSecretsExpression

Impact

This is a security-only fix with no breaking changes:

• ✅ Error messages remain informative and helpful for debugging
• ✅ Validation logic is unchanged
• ✅ API/interface is unchanged
• ✅ No functional regression
• ⚠️ Error messages no longer include the actual value (intentional security improvement)

References

• CodeQL Rule: go/clear-text-logging
• CWE-312: Cleartext Storage of Sensitive Information
• CWE-315: Cleartext Storage of Sensitive Information in a Cookie
• CWE-359: Exposure of Private Personal Information to an Unauthorized Actor
• OWASP: Information Exposure Through an Error Message

AI generated by Security Fix PR